Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security should not reload files that haven't changed #50207

Merged
merged 4 commits into from
Jan 6, 2020
Merged

Security should not reload files that haven't changed #50207

merged 4 commits into from
Jan 6, 2020

Conversation

AntonShuvaev
Copy link
Contributor

In security we currently monitor a set of files for changes:

- config/role_mapping.yml (or alternative configured path)
- config/roles.yml
- config/users
- config/users_roles

This commit prevents unnecessary reloading when the file change actually doesn't change the internal structure.

closes #50063

Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @J-Bean, This is great.
I've done a first pass and left a few comments so that you can get some feedback straight-away. One of us will do a more thorough review over the next few days.

@tvernum
Copy link
Contributor

tvernum commented Dec 16, 2019

@elasticmachine OK to test

@tvernum tvernum added :Security/Security Security issues without another label >enhancement v7.6.0 v8.0.0 labels Dec 16, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Security)

@AntonShuvaev
Copy link
Contributor Author

Fixed. Please take a look.

Copy link
Contributor

@albertzaharovits albertzaharovits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution @J-Bean !
LGTM
We should still wait for Tim to take another pass.

@tvernum tvernum self-requested a review January 2, 2020 06:42
Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tvernum
Copy link
Contributor

tvernum commented Jan 6, 2020

@elasticmachine update branch

@tvernum tvernum self-assigned this Jan 6, 2020
@tvernum tvernum merged commit 3184a89 into elastic:master Jan 6, 2020
@AntonShuvaev AntonShuvaev deleted the not_reload_files_if_not_changed branch January 6, 2020 17:29
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Jan 8, 2020
In security we currently monitor a set of files for changes:

- config/role_mapping.yml (or alternative configured path)
- config/roles.yml
- config/users
- config/users_roles

This commit prevents unnecessary reloading when the file change actually doesn't change the internal structure.

Backport of: elastic#50207

Co-authored-by: Anton Shuvaev <anton.shuvaev91@gmail.com>
tvernum added a commit that referenced this pull request Jan 8, 2020
In security we currently monitor a set of files for changes:

- config/role_mapping.yml (or alternative configured path)
- config/roles.yml
- config/users
- config/users_roles

This commit prevents unnecessary reloading when the file change actually doesn't change the internal structure.

Backport of: #50207

Co-authored-by: Anton Shuvaev <anton.shuvaev91@gmail.com>
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
In security we currently monitor a set of files for changes:

- config/role_mapping.yml (or alternative configured path)
- config/roles.yml
- config/users
- config/users_roles

This commit prevents unnecessary reloading when the file change actually doesn't change the internal structure.

Closes: elastic#50063

Co-authored-by: Anton Shuvaev <anton.shuvaev91@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Security should not reload files that haven't changed
5 participants